Skip to content

Conversation

@skayliu
Copy link
Contributor

@skayliu skayliu commented Sep 19, 2024

Description

Refactor: extend is empty() to check if a string is empty

Related issues

closes #

@skayliu skayliu requested a review from saig0 as a code owner September 19, 2024 06:24
@saig0
Copy link
Member

saig0 commented Sep 20, 2024

@skayliu thank you for taking the time to contribute. 🙇

First, I would like to clarify if we need the function is empty(string) for a string value. I would assume that we would cover the use cases with the new function is blank(string) (ref).

Why do you see a need for this function?
Please describe your use case.

@skayliu
Copy link
Contributor Author

skayliu commented Sep 20, 2024

@skayliu thank you for taking the time to contribute. 🙇

First, I would like to clarify if we need the function is empty(string) for a string value. I would assume that we would cover the use cases with the new function is blank(string) (ref).

Why do you see a need for this function? Please describe your use case.

Because is empty(string) is different from is blank(string) , so i think we need it.

@saig0
Copy link
Member

saig0 commented Sep 20, 2024

Because is empty(string) is different from is blank(string) , so i think we need it.

Yes, the behavior differs in the following cases:

is blank("")     // true
is empty("")     // true 

is blank(" ")    // true
is empty(" ")    // false 

However, I challenge that we need both functions. Please come up with a case where you need is empty() and can't use is blank()?


Reasoning:

  • Adding a new function increases the maintenance of our tools and the complexity of the language.
  • We aim to add all functions to the DMN standard. The OMG needs to approve the proposal. So, we should provide good arguments for it.

@skayliu
Copy link
Contributor Author

skayliu commented Sep 23, 2024

@saig0, Let's close this PR for less good reason.

@skayliu skayliu closed this Sep 23, 2024
@skayliu skayliu deleted the skayliu-extend-is-empty branch September 23, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants